Skip to content

fix(datepicker): update subscriptions of datepicker-inline and daterangepicker-inline#6201

Open
JoschuaSchneider wants to merge 3 commits intovalor-software:developmentfrom
JoschuaSchneider:fix-datepicker-inline-subscriptions
Open

fix(datepicker): update subscriptions of datepicker-inline and daterangepicker-inline#6201
JoschuaSchneider wants to merge 3 commits intovalor-software:developmentfrom
JoschuaSchneider:fix-datepicker-inline-subscriptions

Conversation

@JoschuaSchneider
Copy link
Contributor

@JoschuaSchneider JoschuaSchneider commented Jul 8, 2021

Context

Both datepicker-inline and daterangepicker-inline don't update their subscriptions to the datepicker instance after calling setConfig.
This causes the datepicker to not fire bsValueChanges after one of the datepickers inputs have changed.

Changes

  • Introduce updateSubscriptions method to unsubscribe from previous subscriptions and to create new subscriptions on new _datepickerRef.instance
  • Reduce calls to setConfig by introducing a shouldSetConfig flag, thereby batching complex updates together

Closes #5888 where EventEmitter does not fire after Input changes

PR Checklist

Before creating new PR, please take a look at checklist below to make sure that you've done everything that needs to be done before we can merge it.

  • read and followed the CONTRIBUTING.md guide.
  • built and tested the changes locally.
  • added/updated tests.
  • added/updated API documentation.
  • added/updated demos.

JoschuaSchneider and others added 2 commits July 8, 2021 19:09
@JoschuaSchneider
fix(datepicker): update subscriptions of datepicker-inline and datera…
6441d26 
…ngepicker-inline

Introduce updateSubscriptions method to unsubscribe from previous subscriptions and to create new subscriptions on new _datepickerRef.instance
Reduce calls to setConfig by introducing a shouldSetConfig flag, thereby batching complex updates together

Closes valor-software#5888 where EventEmitter does not fire after Input changes
@daniloff200
Merge branch 'development' into fix-datepicker-inline-subscriptions
7b69d69
@codecov
Copy link

codecov bot commented Jul 29, 2021 •
edited
Loading

Codecov Report

Merging #6201 (b22cff9) into development (e3b2d7d) will decrease coverage by 0.04%.
The diff coverage is 36.36%.

❗ Current head b22cff9 differs from pull request most recent head c333dc7. Consider uploading reports for the commit c333dc7 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           development    #6201      +/-   ##
===============================================
- Coverage        77.52%   77.47%   -0.05%     
===============================================
  Files              302      302              
  Lines            10549    10566      +17     
  Branches          2583     2587       +4     
===============================================
+ Hits              8178     8186       +8     
- Misses            2361     2370       +9     
  Partials            10       10
Impacted Files Coverage Δ
src/datepicker/bs-datepicker-inline.component.ts 53.33% <36.36%> (+0.24%) ⬆️
.../datepicker/bs-daterangepicker-inline.component.ts 59.30% <36.36%> (+0.32%) ⬆️
src/chronos/i18n/pl.ts 81.66% <0.00%> (-3.34%) ⬇️
src/chronos/i18n/cs.ts 87.64% <0.00%> (ø)
src/chronos/i18n/sk.ts 85.13% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3b2d7d...c333dc7. Read the comment docs.

@cypress
Copy link

cypress bot commented Jul 29, 2021 •
edited
Loading


Test summary

818 • 12 • 14 • 0

Run details
Project ngx-bootstrap
Status Failed
Commit b22cff9 ℹ️
Started Aug 20, 2021 10:24 AM
Ended Aug 20, 2021 10:37 AM
Duration 13:05 💡
OS Linux Ubuntu - 20.04
Browser Electron 87

View run in Cypress Dashboard ➡️

Failures

modals_service_page_spec.ts 3 Failed
1 Modals demo page testing suite: Service examples > Component modals with interceptor > when user clicks on "Close" button, triggers the interceptor and doesn't close the modal
2 Modals demo page testing suite: Service examples > Component modals with interceptor > when user clicks on "Yes" button, closes the modal
3 Modals demo page testing suite: Service examples > Component modals with interceptor > when user clicks on "No" button, doesn't close the modal
carousel_page_spec.ts 5 Failed
1 Carousel page testing suite > Slide changed event > example contains slides, indicators, left and right controls and "Slide has been switched: 0"
2 Carousel page testing suite > Slide changed event > when user click on left arrow - info changed to "Slide has been switched: 2"
3 Carousel page testing suite > Slide changed event > when user click on left arrow again - info changed to "Slide has been switched: 1"
4 Carousel page testing suite > Slide changed event > when user click on right arrow - info changed to "Slide has been switched: 1"
5 Carousel page testing suite > Slide changed event > when user click on right arrow again - info changed to "Slide has been switched: 2"
pagination_page_spec.ts 1 Failed
1 Pagination demo page testing suite > Pager > example contains: 7 pages, Next - active, Previous - active, 4th page active
buttons_page_spec.ts 1 Failed
1 Buttons page testing suite > Buttons Custom checkbox value example > when user clicks on it, btn become inactive and custom checkbox value should be "0"
This comment includes only the first 10 test failures. See all 12 failures in the Cypress Dashboard.

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@JoschuaSchneider JoschuaSchneider mentioned this pull request Aug 7, 2021
Open
@evilstiefel evilstiefel mentioned this pull request Aug 12, 2021
Closed
@rfrcarvalho
Copy link

rfrcarvalho commented Oct 5, 2021

Does anyone have an ETA on this PR being approved and merged? This is currently blocking my app from being deployed as the inline calendar has these bugs.

@JoschuaSchneider
Copy link
Contributor Author

JoschuaSchneider commented Apr 20, 2022

Hey, I would love to reduce the amount of work you have to deal with on this PR.
Would resolving the conflicts with the current development branch do any good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp(datepicker)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Using minDate input for the datepicker fails to fire the bsValueChange after first date clicked in v5.60+

5 participants

@JoschuaSchneider @rfrcarvalho @apotapcukv @daniloff200 @valorkin

Comments